chore: add ruff-extra-rules linter/formatter - #410
Conversation
e7a0575 to
a481f2d
Compare
| [tool.ruff-extra-rules] | ||
| fix = true | ||
|
|
||
| [tool.ruff-extra-rules.per-file-ignores] |
There was a problem hiding this comment.
I ran this locally without the per-file ignores and didn't see any changes; did something previously get flagged, or is this just pre-emptive?
There was a problem hiding this comment.
I submitted https://github.com/requests-cache/aiohttp-client-cache/pull/406/commits beforehand, which fixed every finding that didn't touch the public interface. This PR ended up as just the bare pre-commit hook addition on top of that.
The four remaining validate-function-name (TR4) findings are on public library functions, and they're what the per-file-ignores block exists for — the comment right above it explains why: renaming a public function isn't worth breaking the interface now, so it's deferred to the next major release. So no, it isn't pre-emptive.
Removing that block and running prek run -a on this branch reproduces exactly those four:
prek output
Extra Python rule checks (ruff-extra-rules)..............................Failed
- hook id: ruff-extra-rules
- description: Run multiple AST-based checks in a single pass for improved performance. Excludes redundant-type-conversion (TR6) -- see the ruff-extra-rules-ty hook
- exit code: 1
aiohttp_client_cache/cache_control.py:152:1: TR4: Function 'get_expiration_datetime' should be renamed to 'calculate_expiration_datetime' (aggregates or computes a summary)
aiohttp_client_cache/cache_control.py:168:1: TR4: Function 'get_cache_directives' should be renamed to 'extract_cache_directives' (extracts/collects data (returns list/dict))
examples/precache.py:45:1: TR4: Function 'get_page_links' should be renamed to 'extract_page_links' (extracts/collects data (returns list/dict))
aiohttp_client_cache/backends/base.py:274:1: TR4: Function 'get_urls' should be renamed to 'iter_urls' (generator/iterator)
That's the opposite of what you saw. You already approved and said to go ahead and merge, so I'm not blocking on this — but I'd still like to understand the discrepancy: could you share the exact edit you made to pyproject.toml and confirm you ran prek run -a (not, say, ruff-extra-rules scoped to changed files only)?
There was a problem hiding this comment.
I just tested again: checked out the main branch, removed the per-file-ignores block, and ran prek run -a, but it passed with no warnings. Here's the verbose pre-commit output, although I don't think it contains any useful info:
prek run ruff-extra-rules -a -vv
2026-08-27T21:48:01.130687Z DEBUG prek: 0.2.27
2026-08-27T21:48:01.130720Z DEBUG Args: ["prek", "run", "ruff-extra-rules", "-a", "-vv"]
2026-08-27T21:48:01.132585Z DEBUG Git root: /home/jcook/workspace/aiohttp-client-cache
2026-08-27T21:48:01.132603Z DEBUG Found workspace root at `/home/jcook/workspace/aiohttp-client-cache`
2026-08-27T21:48:01.132645Z DEBUG Performing fresh workspace discovery
2026-08-27T21:48:01.133514Z DEBUG Loading project configuration path=.pre-commit-config.yaml
2026-08-27T21:48:01.135920Z DEBUG Acquired lock resource="store"
2026-08-27T21:48:01.137308Z DEBUG Cloning repo target=/home/jcook/.cache/prek/scratch/.tmpmYscWM repo=https://github.com/alessio-locatelli/ruff-extra-rules@v0.2.2
2026-08-27T21:48:03.621505Z DEBUG Hooks going to run: ["ruff-extra-rules"]
2026-08-27T21:48:03.621686Z DEBUG Found uv in PATH: /home/jcook/.local/bin/uv
2026-08-27T21:48:03.627455Z DEBUG Installing environment hook=ruff-extra-rules target=/home/jcook/.cache/prek/hooks/python-SXrTtyfVtTQFsk5Wh5Gn
2026-08-27T21:48:03.757511Z DEBUG Venv created successfully with no downloads: `/home/jcook/.cache/prek/hooks/python-SXrTtyfVtTQFsk5Wh5Gn`
2026-08-27T21:48:04.670205Z DEBUG Installed hook `ruff-extra-rules` in `/home/jcook/.cache/prek/hooks/python-SXrTtyfVtTQFsk5Wh5Gn`
2026-08-27T21:48:04.672163Z DEBUG All files in the workspace: 79
2026-08-27T21:48:04.674405Z DEBUG Running priority group with priority 10 with concurrency 8: ["ruff-extra-rules"]
Extra Python rule checks (ruff-extra-rules)..............................Passed
- hook id: ruff-extra-rules
- duration: 0.19sI tested it in CI (logs here) and got the expected warnings, so it must be something with my local dev environment.
JWCook
left a comment
JWCook
left a comment
There was a problem hiding this comment.
Interesting! So far I've just looked over your docs and didn't dive into the code, but ruff-extra-rules looks carefully thought out. I don't think I've seen semantic analysis in a linter before (redundant-assignment/TR5 rule). TR3 looks useful; that one has bitten me before.
For rule TR7, how does it identify which line is the relevant expression that a comment belongs to, if there are multiple expressions within the bracket?
No objections here; go ahead and merge this if you'd like.
|
Thanks for taking a look! Background on TR7: While working on several large codebases, I observed that enforcing the configured line-length limit caused Black and Ruff to automatically reformat inline comments associated with specific parameters by relocating them to the closing bracket. This significantly reduced code readability and necessitated manual inspection via On TR7: it's purely positional, no attempt to resolve which argument the comment describes.
The Yes, Glad TR3 and TR5 landed well — I'll go ahead and merge this, thanks for the review. |
2 participants
Background: During code review, I've noticed that I repeat the same feedback for both coding agents and human developers. Some of these rules are either not yet implemented in linters or still under development, so I decided to fill this gap.
I'm flexible on the outcome:
If there will be a green light on this, I would like to submit an equivalent PR for https://github.com/requests-cache/requests-cache/